AMW-590 Collection spelling and grammar checking for PRs by RanabirChakraborty · Pull Request #24 · ansible-middleware/github-actions

RanabirChakraborty · 2026-07-10T07:57:23Z

Issue: http://redhat.atlassian.net/browse/AMW-590

iweiss · 2026-07-10T08:47:41Z

A couple of notes:

This should be documented in README.md with how to use this action, per-project settings, global settings, etc.
All files should be spell and grammar checked, not only .md, .rst and task names
You are excluding Andrew's name. If it gets flagged, maybe we need a base English dictionary

iweiss · 2026-07-10T08:41:49Z

+
+
+def _load_accepted_terms():
+    """Load accepted spelling terms from the shared Vale vocabulary file."""


Maybe merged instead of shared?

iweiss · 2026-07-10T08:43:13Z

+SKIP_RULES = {
+    "write-good.TooWordy",
+    "write-good.Weasel",
+}


Couldn't this be part of a shared vale.ini that can be overridden by a project's own vale.ini?

RanabirChakraborty · 2026-07-11T16:08:08Z

@iweiss Thanks for the suggestions, Fixed it accordingly.

iweiss · 2026-07-13T05:32:10Z

+        current = parent
+
+
+VALE_CONFIG = _find_vale_config()


What happens if the file isn't found?

Currently it silently returns a nonexistent path. We can add an error msg to handle this and also in the README we have mentioned that in every collection root path we need to have .vale.ini file present.

Any error should be raised to avoid confusion, as a warning or error depending on the context.

thanks, the FileNotFoundError was already raised inside the function but was called at module level, so a missing .vale.ini would surface as a raw Python traceback in CI. Using try/except to catch it and print a clean Error msg to stderr before exiting with code.

iweiss · 2026-07-13T05:39:45Z

+    with tempfile.NamedTemporaryFile(
+        mode='w', suffix='.md', dir=PROJECT_ROOT, delete=False
+    ) as tmp:
+        tmp.write(clean + '\n')


What is being accomplished by writing this temporary file?

Actually Vale's CLI only accepts file paths, it has no stdin mode. Adding a comment here.

Then aren't we using it wrong?

You're right, the .md temp file should sit beside the config, not in project root path. _build_tasks_config() already does this correctly for the .ini file. Fixed to derive config_dir from the passed config path and use that as the dir for the .md temp file too.

iweiss · 2026-07-13T06:08:21Z

+FQCN
+Jinja
+jinja
+playbook


Isn't this in the standard English dictionary?

iweiss · 2026-07-13T06:08:33Z

+colocated
+multicast
+infinispan
+idempotency


iweiss · 2026-07-13T06:08:44Z

+utils
+repo
+repos
+boolean


iweiss · 2026-07-13T06:12:56Z

+It runs two checks in every pull request:
+
+1. **Prose files** — Vale lints all Markdown and RST files in the collection using the styles configured in the project's `.vale.ini`.
+2. **Ansible YAML name fields** — A Python script extracts `name:` fields from task files, handler files, playbooks, and Molecule scenarios and runs them through Vale individually, suppressing rules that are too noisy for short imperative strings.


iweiss · 2026-07-13T06:13:39Z

+Spell and grammar checking for ansible-middleware collections using [Vale](https://vale.sh/).
+It runs two checks in every pull request:
+
+1. **Prose files** — Vale lints all Markdown and RST files in the collection using the styles configured in the project's `.vale.ini`.


I don't see a .vale.ini in this PR. Is this on purpose?

I think .vale.ini should be independent to the collection, that's why haven't provided any .vale.ini here, we can add one in the collection root path. Also added an example in README.

iweiss · 2026-07-13T06:15:12Z

+    name: Spelling and Grammar check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4


Always suggest and use the latest version of an action, even in examples.

iweiss · 2026-07-13T06:18:23Z

@@ -0,0 +1,106 @@
+Ranabir


Can't the authors key/array be excluded from checking?

Removed it.

iweiss · 2026-07-13T06:18:57Z

+TokenIgnores = (?:[a-z][a-z\d]*_)+[a-z\d_]+
+```
+
+`TokenIgnores` is important: it prevents Vale from spell-checking Ansible `snake_case` variable names inside prose.


What do you mean by "inside prose" in this context?

like for this case, jboss_home appears within a sentence in a Markdown or RST file. Vale would split it at underscores and flag jboss and home separately as misspelled words. Inside prose means the underscored vars. Explained in the comment as well.

iweiss · 2026-07-21T04:48:21Z

+        current = parent
+
+
+VALE_CONFIG = _find_vale_config()


Any error should be raised to avoid confusion, as a warning or error depending on the context.

iweiss · 2026-07-21T04:49:39Z

+    with tempfile.NamedTemporaryFile(
+        mode='w', suffix='.md', dir=PROJECT_ROOT, delete=False
+    ) as tmp:
+        tmp.write(clean + '\n')


Then aren't we using it wrong?

iweiss · 2026-07-21T04:50:35Z

+        os.unlink(tasks_config)
+
+    if found_issues:
+        sys.exit(1)


Why not print what the issues are?

iweiss · 2026-07-21T04:53:20Z

+# .github/vale-tasks.ini — project-specific task name rule tuning
+write-good.TooWordy = NO
+write-good.Weasel = NO
+proselint.LGBTTerms = NO


I'd recommend removing this line

iweiss · 2026-07-21T04:54:23Z

+    # Replace snake_case tokens (e.g. jboss_home) with a placeholder so Vale
+    # does not flag each underscore-joined segment as a misspelled word.
+    clean = re.sub(r'\b(?:[a-z][a-z\d]*_)+[a-z\d_]+\b', 'VARNAME', clean)
+


What happens if someone writes jbos_home?

thanks, so I'm thinking if we can do something like this, we can add both the project's merged vocabulary (.github/styles/config/vocabularies/Base/accept.txt) which is already there and also add the support of the shared action vocabulary (.github/vale-lint/vocabularies/accept.txt), Now _load_accepted_terms() should load both vocabularies (shared action + project) so segment lookups are complete.

Replace the blanket snake_case regex substitution with a per-token check: split the token on _, and only replace with VARNAME if every segment is in the combined accepted terms set. Otherwise leave it as-is so Vale sees the bad segment (e.g. jbos from jbos_home). wdyt?

Doesn't it do this already, #24 (comment)?

yes, so even if we use jbos_home it detect it and throws an error as flags jbos as a spelling error. I guess this is what we wanted it to do.

iweiss · 2026-07-23T04:40:57Z

+    name: Spelling and Grammar check
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v7


v7.0.1 is the latest, https://github.com/actions/checkout/releases

@v7 is a floating tag that GH keeps pointed at the latest v7.x.x patch (currently v7.0.1), so users always get the latest stable release without manually updating it. Using @v7.0.1 would pin to a specific patch.

RanabirChakraborty mentioned this pull request Jul 10, 2026

AMW-590 Collection spelling and grammar checking for PRs ansible-middleware/wildfly#383

Open

RanabirChakraborty requested review from hcherukuri and iweiss July 10, 2026 07:59

iweiss reviewed Jul 10, 2026

View reviewed changes

RanabirChakraborty force-pushed the AMW-590 branch 2 times, most recently from 5391576 to 00a2a10 Compare July 11, 2026 16:07

iweiss reviewed Jul 13, 2026

View reviewed changes

RanabirChakraborty force-pushed the AMW-590 branch from 00a2a10 to 9880727 Compare July 14, 2026 17:48

RanabirChakraborty requested a review from iweiss July 14, 2026 17:49

iweiss reviewed Jul 21, 2026

View reviewed changes

iweiss reviewed Jul 23, 2026

View reviewed changes

AMW-590 Collection spelling and grammar checking for PRs

cf9e424

RanabirChakraborty force-pushed the AMW-590 branch from 9880727 to cf9e424 Compare July 23, 2026 09:18

RanabirChakraborty requested a review from iweiss July 23, 2026 09:18



		def _load_accepted_terms():
		"""Load accepted spelling terms from the shared Vale vocabulary file."""

Uh oh!

Conversation

RanabirChakraborty commented Jul 10, 2026

Uh oh!

iweiss commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RanabirChakraborty commented Jul 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!